fix(gooddata-eval): keep retrying create_metric result, not the first failed one - #1753

Summary

• _extract_metric_from_turn (conversation.py) returned the result of the first create_metric tool call in a turn, regardless of isError.
• When the agent self-corrects an invalid MAQL by retrying create_metric within the same turn, this captured the failed first attempt's error payload instead of the successful retry's payload.
• turn_outputs["create_metric"] then had no metric_id, so any later fixture turn referencing $ref:create_metric.* raised inside _resolve_refs and was silently [SKIP]ped — never sent to the server at all.

Follow-up review (see below) found the same bug pattern, and two related gaps, on the same code path:

• metric_skill._extract_metric_result was an unfixed copy of the same bug, with worse consequences: a failed-then-retried create_metric reported maql_correct=False even though the metric was created, and leaked the created metric into the shared workspace (nothing tracked its id for cleanup).
• conversation._check_output_present's metric branch only checked that create_metric was called, not that it succeeded — a turn where every attempt failed was marked successful, misdirecting debugging to the next (skipped) turn instead of the one that actually failed.
• The extractor didn't guard against a non-dict or empty tool-result payload, which downstream code calls .get() on unconditionally.
• The choice of which call to prefer when a turn has two independent successful create_metric calls (not a retry) was made silently, with no test pinning it.

Root cause

Reproduced live in CI (agent_conversations, "Units Per Transaction" fixture):

• Trace: https://us.cloud.langfuse.com/project/cm71tjc2a0007ad07qkyw5x9q/traces/836a5112a397495fef7692ef7caffce6
• 1st create_metric call: malformed MAQL (WHERE ... = "Processed" / {metric/net_orders}) → AFM 400 Numeric expression with invalid types.
• 2nd create_metric call (~9s later, same turn): corrected MAQL → succeeded.
• The extractor returned on the first match, so turn_outputs/metric_id_to_delete held the error payload instead of the metric result.

Fix

• Consolidated on metric_skill._extract_metric_result as the single implementation (conversation.py now imports it instead of keeping its own copy).
• It now: skips a create_metric result when the payload is an error, not a dict, or empty; and prefers the most recent successful call, consistent with kda_skill's existing "last wins" pairing for retries.
• conversation._check_output_present's metric branch now requires an actually-extracted result, not just any create_metric call.
• _execute_single_metric_run now tracks every metric id any create_metric call produced (via the existing _extract_created_metric_ids) and deletes all of them, instead of only the one derived from the (possibly failed) primary candidate.

evaluators/metric_skill.py's own _find_create_metric has the same first-match pattern, but is only reachable from gooddata-eval's standalone CLI (cli/agentic_runner.py), not from the ai-agent-tests-staging.yml pipeline — left as-is since it doesn't affect current CI.

Test plan

• test_extract_metric_result_* (5 tests, test_agentic_metric_skill.py): failed-then-retry, all-failed, failed-after-success, non-dict payload, empty payload, most-recent-of-two-successes.
• test_run_agentic_metric_skill_deletes_the_metric_created_by_a_self_corrected_retry: end-to-end, asserts the metric the retry created is the one actually deleted.
• test_run_agentic_conversation_sends_the_next_turn_after_a_self_corrected_retry: end-to-end, asserts turn 2's message is actually sent (send_message.call_count == 2), not just that the extractor returns the right value.
• uv run pytest packages/gooddata-eval/tests/test_agentic_conversation.py packages/gooddata-eval/tests/test_agentic_metric_skill.py — 48/48 passed.
• make lint / make format / make types (gooddata-eval) — all clean.
• Full tests/ suite run for regressions: 9 pre-existing failures (ModuleNotFoundError: No module named 'openai', missing [llm-judge] extra in this local env) confirmed present on origin/master too, unrelated to this change.

JIRA: QA-29053

Summary by CodeRabbit

• Bug Fixes

  • Improved metric creation retry handling by ignoring failed, empty, or invalid attempts.
  • The latest successful metric result is now selected correctly.
  • Conversations continue successfully after a self-corrected metric retry.
  • All metrics created during retries are cleaned up.
  • Returns no metric when all creation attempts fail.

• Tests

  • Added regression coverage for failed, malformed, and empty metric results.
  • Added end-to-end coverage for self-correcting metric retries and cleanup.